[WIP,perf] A structure for keeping conditionally extracted syntax contexts #151491
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-author
Contributor
Author
|
The The downside is that we need to unpack the span to read some data from it, and pack it back if we need a modifying operation. Ideally we would keep spans packed only in the necessary data structures, but worked with unpacked spans in all places where we need to execute some logic on them. Individual larger unpacked spans on stack don't pose any problems. Name resolution in particular needs a several "partially unpacked indentifier" structures, to work with this in both fast and principled way. For example, something like struct Macros20NormalizedIdentKey {
name: Symbol,
ctxt: Macros20NormalizedSyntaxContext, // unpacked normalized syntax context
}which would serve as a key for names planted in modules (and other "name containers" like preludes). Also something like struct AnotherIdent {
name: Symbol,
ctxt: SyntaxContext, // unpacked possibly normalized syntax context
// loses information compared to original span during normalization
// so it's incorrect to use it for edition checks, and suboptimal to use it for diagnostics (macro backtrace will be incomplete)
orig_span: Span, // original non-normalized identifier span, used for diagnostics and edition checks
} |
rustbot
removed
the
S-waiting-on-author
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
rust-bors bot
pushed a commit
that referenced
this pull request
Jan 28, 2026
resolve: Replace `Macros20NormalizedIdent` with `IdentKey` This is a continuation of #150741 and #150982 based on the ideas from #151491 (comment). Before this PR `Macros20NormalizedIdent` was used as a key in various "identifier -> its resolution" maps in `rustc_resolve`. `Macros20NormalizedIdent` is a newtype around `Ident` in which `SyntaxContext` (packed inside `Span`) is guaranteed to be normalized using `normalize_to_macros_2_0`. This type is also used in a number of functions looking up identifiers in those maps. `Macros20NormalizedIdent` still contains span locations, which are useless and ignored during hash map lookups and comparisons due to `Ident`'s special `PartialEq` and `Hash` impls. This PR replaces `Macros20NormalizedIdent` with a new type called `IdentKey`, which contains only a symbol and a normalized unpacked syntax context. (E.g. `IdentKey` == `Macros20NormalizedIdent` minus span locations.) So we avoid keeping additional data and doing some syntax context packing/unpacking. Along with `IdentKey` you can often see `orig_ident_span: Span` being passed around. This is an unnormalized span of the original `Ident` from which `IdentKey` was obtained. It is not used in map keys, but it is used in a number of other scenarios: - diagnostics - edition checks - `allow_unstable` checks This is because `normalize_to_macros_2_0` normalization is lossy and the normalized spans / syntax contexts no longer contain parts of macro backtraces, while the original span contains everything.
github-actions bot
pushed a commit
to rust-lang/rust-analyzer
that referenced
this pull request
Jan 29, 2026
resolve: Replace `Macros20NormalizedIdent` with `IdentKey` This is a continuation of rust-lang/rust#150741 and rust-lang/rust#150982 based on the ideas from rust-lang/rust#151491 (comment). Before this PR `Macros20NormalizedIdent` was used as a key in various "identifier -> its resolution" maps in `rustc_resolve`. `Macros20NormalizedIdent` is a newtype around `Ident` in which `SyntaxContext` (packed inside `Span`) is guaranteed to be normalized using `normalize_to_macros_2_0`. This type is also used in a number of functions looking up identifiers in those maps. `Macros20NormalizedIdent` still contains span locations, which are useless and ignored during hash map lookups and comparisons due to `Ident`'s special `PartialEq` and `Hash` impls. This PR replaces `Macros20NormalizedIdent` with a new type called `IdentKey`, which contains only a symbol and a normalized unpacked syntax context. (E.g. `IdentKey` == `Macros20NormalizedIdent` minus span locations.) So we avoid keeping additional data and doing some syntax context packing/unpacking. Along with `IdentKey` you can often see `orig_ident_span: Span` being passed around. This is an unnormalized span of the original `Ident` from which `IdentKey` was obtained. It is not used in map keys, but it is used in a number of other scenarios: - diagnostics - edition checks - `allow_unstable` checks This is because `normalize_to_macros_2_0` normalization is lossy and the normalized spans / syntax contexts no longer contain parts of macro backtraces, while the original span contains everything.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
An implementation for the suggestion from #150982 (comment).
I won't even benchmark it, just submitting for history.
The problem of having both packed and unpacked spans in different places is real, but this isn't the way to solve it, see some thoughts in the next comment.